refactor(multiple): rework LabelControl across aria components#33146
refactor(multiple): rework LabelControl across aria components#33146adolgachev wants to merge 3 commits intoangular:mainfrom
Conversation
angular-robot
Bot
added
detected: feature
adolgachev
added
dev-app preview
|
Deployed dev-app for a867429 to: https://ng-dev-previews-comp--pr-angular-components-33146-dev-qadmum20.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
adolgachev
force-pushed
the
aria-labels
branch
5 times, most recently
from
3082e63 to
bad3cc9
Compare
There was a problem hiding this comment.
Can we stop the "I want this in my own style" kind of refactoring? I don't see the core functionality has changed after touching almost every single line.
There was a problem hiding this comment.
See the PR comments for the 3 benefits in usage. If we go to the trouble of creating a utility, it should be easy and clear in its usage. Otherwise, could literally just do the logic more simply in the pattern. In this case, it turned out to be premature re-factoring as the LabelControl was not used in any beneficial way previously, just adding to burden.
| }); | ||
|
|
||
| constructor() { | ||
| this._labelControl = new LabelControl({ |
There was a problem hiding this comment.
We should put this in the UI pattern to keep the directive a thin layer between the framework and the core library.
adolgachev
removed
action: review
2 participants
Refactors to make LabelControl easier to use by directives directly:
Added proper usage to AccordionPanel and TabPanel so default labelledby still set properly to the respective AccordionTrigger or Tab, but also could be overridden. Although not recommended to do so.